Skip to content

Fix version-directory cleanup race and run cloudformation-validate in a worker thread - #700

Open
satyakigh wants to merge 8 commits into
mainfrom
errors-db
Open

satyakigh wants to merge 8 commits into
mainfrom
errors-db

Conversation

@satyakigh

@satyakigh satyakigh commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

This change addresses two production reliability issues:

  • Prevent datastore version cleanup from deleting cache directories that another language-server process recently used, and remove unused LMDB cache versions when FileDB is active.
  • Isolate cloudformation-validate in a bounded worker thread so expensive validation cannot block or exhaust the language-server process.

Datastore cleanup

FileDB and LMDB persist data in versioned directories (filedb/v4 and lmdb/v7). Cleanup previously removed older directories without accounting for another IDE window or language-server process still using that version, which caused the files.scan.error spike during the FileDB v3-to-v4 migration.

The updated implementation:

  • Centralizes FileDB/LMDB storage roots, version numbers, version-directory discovery, and cleanup behavior.
  • Writes a .heartbeat in the active FileDB or LMDB version directory at startup and refreshes it every 60 seconds.
  • Runs cleanup two minutes after startup and preserves directories used within the last 24 hours. The heartbeat is authoritative, with directory mtime as the fallback for binaries that predate heartbeat support.
  • Keeps each backend's own current and newer versions while removing unused older versions.
  • When FileDB is active, also removes unused LMDB version directories, including the current LMDB version, because that process is not using LMDB.
  • Isolates listing and per-directory deletion failures so one bad path does not prevent cleanup of other eligible directories.
  • Reports cleanup outcomes using the scoped filedb.oldVersion.cleanup.* and lmdb.oldVersion.cleanup.* metrics.

cloudformation-validate isolation

Large templates with many conditional (Fn::If) properties could exhaust the validation engine's memory. Because the engine ran on the main thread, a large validation could freeze the language server and leave subsequent validations failing.

The validation engine now:

  • Runs in a dedicated worker thread with a 128 MB old-generation heap limit.
  • Bundles the validation worker as a separate webpack entry.
  • Enforces a 30-second validation timeout.
  • Processes one validation at a time without blocking the main thread.
  • Terminates and releases the worker after a crash, exit, timeout, or validation failure, then disables comparisons for the rest of the session.
  • Shuts down asynchronously and rejects pending initialization or validation work with a typed shutdown error.

Worker and retry failures now use typed errors (WorkerExitError, WorkerFailureError, WorkerShutdownError, and RetryError) while preserving the existing user-facing messages. Error mapping recognizes the new worker failures, and the cfn-lint worker uses the same typed worker errors.

Supporting changes

  • Excludes the worker-only entry point from direct coverage instrumentation.
  • Updates package metadata, OpenTelemetry dependency declarations, and the generated dependency lockfile used by the final bundle.

Validation

  • npm run build
  • npm run lint
  • npm run test — 299 test files passed; 5,039 tests passed and 3 skipped
  • Coverage: 91.21% lines, 84.10% branches, 92.61% functions, 91.48% statements

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@satyakigh
satyakigh requested a review from a team as a code owner October 6, 2026 21:30
@satyakigh
satyakigh added this pull request to stack #701 October 7, 2026 20:03
Comment thread tst/unit/services/cfnLint/PyodideWorkerManager.test.ts Fixed
Comment thread tst/unit/services/cfnLint/PyodideWorkerManager.test.ts Fixed
@satyakigh satyakigh changed the title Fix version-directory cleanup race and add error type labels to telemetry Fix version-directory cleanup race and run cloudformation-validate in a worker thread Oct 7, 2026
@github-code-quality

github-code-quality Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/vitest

The overall line coverage in commit ec4c039 in the errors-db branch remains at 91%, unchanged from commit 19e02f7 in the main branch.

Show a line coverage summary of the most impacted files.
File main 19e02f7 errors-db ec4c039 +/-
src/datastore/L...StoreFactory.ts 90% 90% 0%
src/datastore/V...ionDirectory.ts 100% 100% 0%
src/services/cf...orkerManager.ts 87% 87% 0%
src/utils/error...ErrorClasses.ts 100% 100% 0%
src/services/cf...idateService.ts 98% 100% +2%
src/datastore/F...StoreFactory.ts 93% 95% +2%
src/services/cf...lidateEngine.ts 93% 97% +4%

Updated October 14, 2026 02:28 UTC

mrinaudo-aws
mrinaudo-aws previously approved these changes Oct 8, 2026

@mrinaudo-aws mrinaudo-aws left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Base automatically changed from npm-publish to main October 9, 2026 21:44
@satyakigh
satyakigh dismissed mrinaudo-aws’s stale review October 9, 2026 21:44

The merge-base changed after approval.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants